Conversation
There was a problem hiding this comment.
Pull request overview
Updates the OGC/pygeoapi surface to better reflect “actively monitored” wells based on the latest monitoring status, and adds a new project_areas OGC collection backed by group polygon boundaries plus a CLI importer to populate those boundaries.
Changes:
- Update
ogc_actively_monitored_wellsview logic to filter by the latestStatusHistory(“Currently monitored”). - Add
ogc_project_areasview + pygeoapi resource and test coverage for the new collection. - Introduce a CLI command (
import-project-area-boundaries) and implementation to fetch ArcGIS layer features and updateGroup.project_area.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_ogc.py |
Adds DB-backed tests for actively monitored well filtering and validates project_areas collection exposure. |
tests/test_cli_commands.py |
Adds a CLI test covering the project area boundary import flow using monkeypatched HTTP + DB session. |
core/pygeoapi-config.yml |
Registers the new project_areas collection mapped to ogc_project_areas.project_area. |
cli/project_area_import.py |
Implements ArcGIS feature fetch + GeoJSON→WKT conversion and updates matching Group rows. |
cli/cli.py |
Adds the import-project-area-boundaries Typer command that calls the importer and prints a summary. |
alembic/versions/t6u7v8w9x0y1_add_project_areas_ogc_view.py |
Creates ogc_project_areas view over groups with non-null project_area. |
alembic/versions/r2s3t4u5v6w7_add_actively_monitored_wells_pygeoapi_materialized_view.py |
Updates ogc_actively_monitored_wells view definition to join latest monitoring status. |
| assert "Fetched 2 feature(s)." in result.output | ||
| assert "Matched 1 group row(s)." in result.output | ||
| assert "Updated 1 group project area(s)." in result.output | ||
| assert "Unmatched locations: Missing Group" in result.output |
There was a problem hiding this comment.
The CLI command prints the “Unmatched locations …” line using typer.echo(..., err=True), which goes to stderr. This test asserts the message in result.output (stdout), so it will fail when stderr is not mixed into output. Update the test to assert against result.stderr (or create the CliRunner with mix_stderr=True and keep the stdout assertion).
| assert "Unmatched locations: Missing Group" in result.output | |
| assert "Unmatched locations: Missing Group" in result.stderr |
| @cli.command("import-project-area-boundaries") | ||
| def import_project_area_boundaries_command( | ||
| layer_url: str = typer.Option( | ||
| ( | ||
| "https://maps.nmt.edu/server/rest/services/Water/" | ||
| "Water_Resources/MapServer/17" | ||
| ), | ||
| "--layer-url", | ||
| help="ArcGIS Feature Layer URL for project area boundaries.", | ||
| ), | ||
| ): | ||
| from cli.project_area_import import import_project_area_boundaries | ||
|
|
||
| result = import_project_area_boundaries(layer_url=layer_url) | ||
| typer.echo(f"Fetched {result.fetched} feature(s).") | ||
| typer.echo(f"Matched {result.matched} group row(s).") | ||
| typer.echo(f"Updated {result.updated} group project area(s).") |
There was a problem hiding this comment.
The PR description is still the default template ("Use bullet points here") and doesn’t describe the why/how for these changes (new OGC view semantics and new project_areas collection/CLI importer). Please update the PR description with a brief rationale and implementation summary so reviewers know the intended behavior and rollout considerations.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c4a8337e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| FROM status_history AS sh | ||
| WHERE | ||
| sh.target_table = 'thing' | ||
| AND sh.status_type = 'Monitoring Status' | ||
| ORDER BY sh.target_id, sh.start_date DESC, sh.id DESC |
There was a problem hiding this comment.
Thing.monitoring_status only treats rows with end_date is None as current (services/util.py:260-264), but this CTE now ranks every Monitoring Status row regardless of end_date. For wells whose latest/current status has been closed out without a replacement, ogc_actively_monitored_wells will still be driven by stale history and return the wrong feature set.
Useful? React with 👍 / 👎.
| JOIN latest_monitoring_status AS lms ON lms.thing_id = wws.id | ||
| WHERE lower(trim(g.name)) = 'water level network' | ||
| AND lms.status_value = 'Currently monitored' |
There was a problem hiding this comment.
This behavioral change is being shipped by editing the already-existing Alembic revision r2s3t4u5v6w7 in place. Any database that has already upgraded through that revision will not rerun this file on alembic upgrade head, so long-lived environments keep the old ogc_actively_monitored_wells definition while fresh installs get the new one; the view recreation needs its own new migration.
Useful? React with 👍 / 👎.
| groups = session.scalars( | ||
| select(Group).where( | ||
| func.lower(func.trim(Group.name)) == _normalize_name(location_name) | ||
| ) | ||
| ).all() |
There was a problem hiding this comment.
The importer matches groups only by normalized name, but Group is only unique on (name, group_type) (db/group.py:58-60), and project lookups elsewhere are scoped to group_type == "Monitoring Plan" (services/well_inventory_csv.py:182-189). If another group type shares a project name, this command will overwrite project_area on every matching row instead of just the project group.
Useful? React with 👍 / 👎.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?